Refine README as an engineering project profile - #11
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b8ed4d67d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| ```bash | ||
| PYTHONPATH=src python -m unittest discover -s tests | ||
| python -m ruff check src/app tests scripts |
There was a problem hiding this comment.
Install the dev extra before the Ruff check
In a clean venv following the documented setup, python -m pip install -e . installs only the runtime dependencies; ruff is declared only in the dev optional dependency/requirements-dev path. This validation command will therefore fail with No module named ruff unless the reader already has Ruff installed globally, so the reproduce steps should install -e '.[dev]' or add an explicit dev-dependency step before running it.
Useful? React with 👍 / 👎.
| [package smoke](src/app/demo/package_smoke.py) exercises fixture load, reference correction, export, and reload from | ||
| the frozen executable and records `live_hardware_used: false`. This is packaging evidence, not a certified release |
There was a problem hiding this comment.
Do not claim package-smoke reference correction
The package smoke currently loads the reference and checks interpolation, but it exports loaded.result directly and never applies the reference to the fixture points. In contexts where this README is used as validation evidence, saying the frozen executable exercises “reference correction” overstates the packaged workflow coverage; either qualify this as reference load/interpolation or extend the smoke to apply and verify correction.
Useful? React with 👍 / 👎.
|
|
||
| ## Safety Notes | ||
| Saving a measurement can produce `*.mat`, `*.csv`, `*.txt`, and optional `*_gain.png` / `*_gain_db.png` plots. | ||
| Exports preserve source, correction mode, point count, timestamp, and the simulated/live boundary when available. |
There was a problem hiding this comment.
Qualify which export formats preserve metadata
This sentence implies every saved export preserves point count and timestamp, but MeasurementExporter only writes those into the MAT metadata_json; CSV has source/boundary/correction plus numeric columns, and TXT has only numeric columns. Users relying on CSV/TXT as analysis-ready artifacts will not get the documented metadata, so the README should limit this claim to MAT/operator receipts or the exporters should add the metadata to those formats.
Useful? React with 👍 / 👎.
| To inspect the workflow without instruments, launch the application and select **Load Demo Fixture**. The console | ||
| will retain the simulated/no-hardware label throughout replay and export. |
There was a problem hiding this comment.
Clarify the fixture must be launched from a resource root
This demo step only works when the app's runtime root contains demo_data, because on_load_demo_fixture builds the fixture path from AppPaths.root_dir (the cwd or AUTO_LOAD_OFF_TEST_ROOT) rather than from installed/bundled package resources. If a reviewer uses the advertised console entry from another directory or sets a separate runtime root for a packaged install, the button shows Demo fixture not found; either document that the app must be launched from the repo root/copy demo_data, or resolve the fixture via the bundled resource path.
Useful? React with 👍 / 👎.
Summary
Validation
PYTHONPATH=src python3 -m unittest discover -s tests(82 tests)python3 -m ruff check src/app tests scriptsgit diff --checkEvidence boundary
Current public validation remains hardware-free. The README does not claim live AWG/oscilloscope, metrology, electrical safety, or production certification.